Skip to content

Add in a legend component with last updated metadata#56

Open
ajistrying wants to merge 4 commits intoperrycate:mainfrom
ajistrying:add-last-updated-ui
Open

Add in a legend component with last updated metadata#56
ajistrying wants to merge 4 commits intoperrycate:mainfrom
ajistrying:add-last-updated-ui

Conversation

@ajistrying
Copy link
Copy Markdown
Contributor

@ajistrying ajistrying commented Feb 16, 2023

Aims to finish up #36

Couple things:

  • Put minimal effort into how it looks in the UI to get it up and working. @perrycate let me know if that's sufficient, the legend should pop up on the bottom left of the map.
  • I had to add map, metadata.updated_at to the dependency array of the useEffect call for the legend component in order to make sure the data was there to render. If there's a better way to write that useEffect call lmk. I'm meh at react, so if I did anything ridiculous I'm down for any and all improvements lol. I'd like to get better at react too 👍

@perrycate
Copy link
Copy Markdown
Owner

Will take a closer look at this soon, probably tomorrow. Thanks!

const Legend = ({metadata}) => {
const map = useMap()
useEffect(() => {
if (map && metadata.updated_at) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to make sure something is actually inside of the metadata object before we go through with adding the legend to the map, otherwise it'll populate prematurely without waiting for the actual data.

Copy link
Copy Markdown
Owner

@perrycate perrycate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hesitant to add something as a "legend" when it's not a real map legend per se - this feels like a bit of a hack. Rather than using built-in leaflet stuff to modify the map itself, what about some sort of overlay? I'm hardly an expert but this feels like something with display property and/or Z-index might be helpful here. Failing that, I think I would prefer to just add an element to the text area than messing with the map, since I suspect that will be simpler to implement. Let me know if you wanna talk more about options!


#mapbox-legend {
background-color: white;
padding: 1em;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's shrink the padding so this takes up less space - maybe something similar to whatever the padding is for the attribution in the other corner?

legend.onAdd = () => {
const div = L.DomUtil.create("div");
div.id = "mapbox-legend"
div.innerHTML = `<h4>Last Updated: ${new Date(metadata.updated_at * 1000).toLocaleString()}</h4>`;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having to manually manipulate innerHTML feels a little suspicious to me. I suspect there may be a better way to do this with the react-leaflet library, but moreso I bet there's a way we can accomplish this without needing to do that, either.

In general one should probably not need to touch innerHTML or similar things inside a react project.

@perrycate
Copy link
Copy Markdown
Owner

I know it's been a couple weeks while you focused on that other PR - feel free to ping me if you have any questions or wanna chat more about this!

@ajistrying
Copy link
Copy Markdown
Contributor Author

Will take a look back at this later today, it's been a busy past few days!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants